-
Notifications
You must be signed in to change notification settings - Fork 26.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace pages-plugin with loader #5994
Conversation
packages/next/build/webpack/loaders/next-client-pages-loader.ts
Outdated
Show resolved
Hide resolved
packages/next/build/webpack/loaders/next-client-pages-loader.ts
Outdated
Show resolved
Hide resolved
Co-Authored-By: timneutkens <tim@timneutkens.nl>
packages/next/build/webpack/loaders/next-client-pages-loader.ts
Outdated
Show resolved
Hide resolved
@timneutkens What are the chances of being able to backport this fix to a 7.0.3 release instead of waiting for 8.0 to be ready? Is the code much different due to other 8.x changes? |
tbh, I hesitate to even use the term "backport" considering that 7.x is very much the current, latest version, even if you may consider it the "previous" version due to your hard work on 8.0 every day 😄 Just wanna make sure we consider people using this in their day-to-day work who may not be able to do a major bump so easily, let alone wait for it to be released. Doesn't just apply to this fix, but anything that in theory could be applied to 7.x and not just 8.0. In many projects it would be customary to keep releasing updates to the current version, even if a major refactor is in the works (like React does for example) Anyway, I'm happy to help make this happen if possible! |
It's not easy / feasible to backport these changes at this point (it would require a tremendous amount of work), version 8 is going to have only slight changes, there's going to be no webpack / babel upgrade like the previous 2 major versions. Only a React upgrade, which is fully backwards compatible. After v8 I'm going to change our release cycle quite a bit, tldr being more release on the stable channel. |
Sounds good! |
is this fixed in v7.0.2? if not, what should I do since the v8 upgrade is a lot of work for us? |
Hi @timneutkens I'm sorry to be the evil's messenger, but I'm using Next 9.* and I'm still facing this issue. It starts to be annoying, and specially important to our team. |
I am still facing the same issue. Some of the links just do not redirect to new pages |
It’s really not useful to reply “same issue” create new issue that has a clear and concise reproduction that allows us to look into it. It’s impossible to investigate otherwise. |
Fixes #5598
Thanks to @malimccalla's clear reproduction I was able to pinpoint the issue that caused the router to hang. It's relatively complex but I'll try to explain.
Next.js has two commands for bundling:
next
(or programmatic with thedev
flag) andbuild
. When runningnext build
we bundle up all pages into the.next
directory and those files are used to serve pages withnext start
.The development mode works differently and is optimized for running only part of your application. We call this on-demand-entries. The way it works is that when you boot up Next.js in dev mode we only compile Next.js's core files. Your pages are not compiled on bootup, instead, when you go to
http://localhost:3000/about
Next.js will injectpages/about.js
(orpages/about/index.js
depending on your dir structure) into webpack, wait for it to compile, and then finish rendering. This is done to make it faster for users to work on only part of the application without having to build the entire application. To give an example zeit.co has 375 pages, meaning that bootup would take a while.On-demand-entries also has a mechanism called "disposal", meaning that if you were on
/about
and you go to/blog
, it will dispose the page if it's not viewed for 60 seconds. If you ever looked at chrome devtools you have probably seenon-demand-entries-ping
executing every 5 seconds, this is basically a keepalive so that disposal knows if a page is still active. In Next.js 8 / currently on canary this ping has been moved to a websocket so we no longer fill your devtools network tab with these pings.The issue described in #5598 was caused by disposal kicking in and removing the page, but the client state not updating correctly to reflect this. So when disposal kicked in the page cache would be flushed, but an instance of the previous page would still exist in webpack's global instance cache, causing the new code that was being loaded not to execute.
This PR simplifies a lot of the HMR logic we had. Previously there were a lot of edge cases that were handled by replacing the component inside the router by sending a message from the hot-reloader server to the client side. I've refactored this to use
module.hot.accept
instead so that there is a single source of truth in replacing the component. Furthermore I've replaced the page registration and hot-self-accept plugins with a single webpack loader that will generate the page code, making it easier to understand/maintain.It's probably good to note that we have integration tests for a lot of HMR cases / HMR error recovery and other edge cases. All tests are still green ✅.